Skip to content

fix(native-eval): benchmark genuine OpenClaw code mode - #62

Open
vincentkoc wants to merge 5 commits into
mainfrom
fix/openclaw-tool-search-explicit-off
Open

fix(native-eval): benchmark genuine OpenClaw code mode#62
vincentkoc wants to merge 5 commits into
mainfrom
fix/openclaw-tool-search-explicit-off

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jul 29, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Makes the OpenClaw native-eval arms represent genuine direct, directory, and
Code Mode execution, then exports and validates their public trajectory bundles
before a run can be scored.

Why?

Fixes #61.

The earlier code arm selected the legacy tool_search_code bridge rather than
OpenClaw Code Mode. The harness also masked setup failures, forced thinking off,
terminated the runtime before lifecycle cleanup settled, and reconstructed
delegated traces from private session files. Those failures made the released
direct/code comparison invalid and disproportionately erased Code Mode's nested
tool calls.

Changes

  • Map direct, directory, and code to explicit, mutually exclusive
    OpenClaw tool surfaces.
  • Propagate the requested reasoning effort through planning, dispatch,
    OpenClaw defaults, subagents, CLI execution, and manifests.
  • Fail setup immediately and let openclaw agent exit naturally.
  • Export root and child trajectories through
    openclaw sessions export-trajectory.
  • Capture child runs at the canonical pre-cleanup terminal hook, including
    nested and repeated runs, without blocking the Gateway.
  • Validate audit initialization, trace/session identity, event counts, terminal
    status, complete Code Mode snapshots, and the exact provider-visible
    exec/wait surface.
  • Reconstruct Code Mode nested calls and complete session-tree usage from the
    public export bundles.
  • Bound child-export stabilization and reject missing or failed evidence rather
    than publishing partial traces.

Tests

  • Blacksmith Testbox full suite: 454 passed, 5 skipped
  • Blacksmith Testbox focused runner suite after final fixes: 61 passed
  • Ruff clean
  • Python compile, git diff --check, generated shell syntax for all three
    modes, and generated audit-plugin node --check
  • Fresh Codex autoreview: clean
  • Fresh matched direct/Code Mode ShellBench batch

The July 29, 2026 released code result remains legacy Tool Search bridge
evidence only. It is not evidence about genuine OpenClaw Code Mode.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 3, 2026, 5:05 AM ET / 09:05 UTC.

ClawSweeper review

What this changes

The PR makes ShellBench’s OpenClaw native-eval runner explicitly select direct, directory, or Code Mode execution and export validated root and child trajectories before benchmark scoring.

Merge readiness

Blocked by patch quality or review findings - 8 items remain

Keep this PR open. It correctly targets a current native-eval gap, but its new validation turns recognized legacy fleet and environment configuration into pre-dispatch failures; the two existing P1 compatibility blockers remain unresolved. Likely related people: Vincent Koc (high confidence), based on the current-main native-eval history and this branch’s commits.

Priority: P2
Reviewed head: f5b31cb1260b631b811bf0ede9f4eba51a94b98c

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The implementation has substantial focused tests, but two current P1 upgrade regressions make the patch not ready to merge.
Proof confidence 🌊 off-meta tidepool Not applicable: The PR is authored by a repository member, so the external-contributor proof gate does not apply; its body reports Testbox suites, while a fresh matched direct/Code-Mode batch remains useful validation for the benchmark claim.
Patch quality 🦪 silver shellfish (2/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The PR is authored by a repository member, so the external-contributor proof gate does not apply; its body reports Testbox suites, while a fresh matched direct/Code-Mode batch remains useful validation for the benchmark claim.
Evidence reviewed 6 items Current main does not implement the proposed mode path: Current main has no implementation of openclaw_tool_mode or SHELLBENCH_OPENCLAW_TOOL_MODE in the native-eval runner or focused tests, so this work is neither implemented nor superseded on main.
Legacy fleet manifests become fatal: The changed plan validation raises for any non-empty legacy openclaw_tool_search_mode instead of translating it to the new explicit field. Existing queued plans therefore fail before benchmark dispatch.
Legacy environment invocation becomes fatal: The changed CLI path raises when SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE is set rather than accepting and normalizing the legacy input, so existing benchmark automation fails before run-spec construction.
Findings 2 actionable findings [P1] Translate retired manifest tool-mode values
[P1] Translate the legacy environment tool-mode input
Security None None.

How this fits together

ShellBench’s native-eval fleet turns stored run plans and environment settings into remote OpenClaw benchmark jobs. Those jobs generate trajectories for the audit and scoring pipeline, so mode selection and export validation determine whether comparisons are valid.

flowchart LR
  A[Run plan and environment] --> B[Fleet validation]
  B --> C[Remote benchmark dispatch]
  C --> D[OpenClaw tool mode]
  D --> E[Public trajectory export]
  E --> F[Trajectory validation]
  F --> G[Benchmark audit and scoring]
Loading

Before merge

  • Translate retired manifest tool-mode values (P1) - Recognized openclaw_tool_search_mode entries in already-created fleet manifests now fail in _validate_plan before dispatch. Translate them to the equivalent explicit openclaw_tool_mode, warn, and add upgrade coverage rather than forcing queued plans to be rewritten.
  • Translate the legacy environment tool-mode input (P1) - Existing callers that set SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE now abort before a run spec exists. Accept recognized legacy values, normalize them to the canonical mode with a deprecation warning, and test the mapping so upgrades preserve benchmark execution.
  • Resolve merge risk (P1) - Merging as written causes older non-empty openclaw_tool_search_mode plan entries and SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE invocations to abort before dispatch, requiring operator intervention during upgrade.
  • Resolve merge risk (P1) - The PR body leaves a fresh matched direct-versus-Code-Mode ShellBench batch unchecked, so the intended end-to-end comparison has not been demonstrated for this revision.
  • Complete next step (P2) - The two blocking defects have a narrow, mechanical compatibility repair that can be attempted on this PR branch without choosing a new product direction.
  • Improve patch quality - Translate legacy manifest and environment values with deprecation warnings.
  • Improve patch quality - Add upgrade tests for legacy disabled, directory, and code inputs.
  • Improve patch quality - Attach a redacted fresh matched direct/Code-Mode batch transcript after the repair.

Findings

  • [P1] Translate retired manifest tool-mode values — scripts/native_eval/fleet.py:447-451
  • [P1] Translate the legacy environment tool-mode input — scripts/native_eval/run_job.py:370-374
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 9 files affected; production +699/-477, tests +1,116/-475 The runner and trajectory rewrite is broad enough that upgrade compatibility needs explicit coverage.

Merge-risk options

Maintainer options:

  1. Preserve legacy benchmark inputs (recommended)
    Translate recognized legacy manifest and environment mode values to the new explicit mode and add focused upgrade tests before merging.
  2. Accept a breaking migration
    Merge strict rejection only if maintainers intentionally require all fleet plans and invocation automation to migrate before the release.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Translate legacy OpenClaw tool-mode manifest and environment inputs with a deprecation warning; add tests for legacy disabled, directory, and code values plus canonical-field precedence.

Technical review

Best possible solution:

Translate recognized legacy mode values to the explicit replacement before validation, emit a deprecation warning, preserve the canonical field through manifests and reruns, and attach a redacted fresh matched benchmark transcript after the compatibility tests pass.

Do we have a high-confidence way to reproduce the issue?

Yes. A stored non-empty openclaw_tool_search_mode reaches the new fleet error, and an invocation with SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE reaches the new run-spec error before dispatch.

Is this the best way to solve the issue?

No. Explicit mode selection is the right direction, but outright rejection of recognized existing inputs is not the narrowest upgrade-safe repair; translation with deprecation preserves existing benchmark automation.

Full review comments:

  • [P1] Translate retired manifest tool-mode values — scripts/native_eval/fleet.py:447-451
    Recognized openclaw_tool_search_mode entries in already-created fleet manifests now fail in _validate_plan before dispatch. Translate them to the equivalent explicit openclaw_tool_mode, warn, and add upgrade coverage rather than forcing queued plans to be rewritten.
    Confidence: 0.99
  • [P1] Translate the legacy environment tool-mode input — scripts/native_eval/run_job.py:370-374
    Existing callers that set SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE now abort before a run spec exists. Accept recognized legacy values, normalize them to the canonical mode with a deprecation warning, and test the mapping so upgrades preserve benchmark execution.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 884dd1bb5511.

Labels

Label justifications:

  • P2: This is a meaningful benchmark-validity correction, but the impact is limited to ShellBench’s native-eval workflow rather than a production availability incident.
  • merge-risk: 🚨 compatibility: The changed validation rejects legacy stored plan and environment inputs before existing benchmark runs can dispatch.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The PR is authored by a repository member, so the external-contributor proof gate does not apply; its body reports Testbox suites, while a fresh matched direct/Code-Mode batch remains useful validation for the benchmark claim.

Evidence

Acceptance criteria:

  • [P1] python -m pytest tests/test_native_eval_fleet.py tests/test_native_eval_runner.py.
  • [P1] git diff --check.

What I checked:

  • Current main does not implement the proposed mode path: Current main has no implementation of openclaw_tool_mode or SHELLBENCH_OPENCLAW_TOOL_MODE in the native-eval runner or focused tests, so this work is neither implemented nor superseded on main. (scripts/native_eval, 884dd1bb5511)
  • Legacy fleet manifests become fatal: The changed plan validation raises for any non-empty legacy openclaw_tool_search_mode instead of translating it to the new explicit field. Existing queued plans therefore fail before benchmark dispatch. (scripts/native_eval/fleet.py:447, f5b31cb1260b)
  • Legacy environment invocation becomes fatal: The changed CLI path raises when SHELLBENCH_OPENCLAW_TOOL_SEARCH_MODE is set rather than accepting and normalizing the legacy input, so existing benchmark automation fails before run-spec construction. (scripts/native_eval/run_job.py:370, f5b31cb1260b)
  • Proposed tests preserve the incompatible behavior: The branch tests rejection of the retired environment variable but do not test translating legacy manifest or environment values, confirming that upgrade compatibility is absent. (tests/test_native_eval_runner.py:687, f5b31cb1260b)
  • Feature-history ownership: Available history shows Vincent Koc introduced the native matrix runner, stabilized its results and traces, and authored current main’s trajectory reconstruction as well as every commit in this PR. (scripts/native_eval/harnesses.py:358, 884dd1bb5511)
  • Default new behavior is explicit direct mode: The branch defaults missing canonical mode to direct and writes tools.toolSearch: false; the blocker is compatibility for explicitly stored or exported legacy inputs, not the fresh-install default. (scripts/native_eval/harnesses.py:367, f5b31cb1260b)

Likely related people:

  • Vincent Koc: Available native-eval history credits Vincent Koc with the matrix runner, trace stabilization, and current-main trajectory reconstruction; the same author created all five commits in this PR. (role: feature owner and recent area contributor; confidence: high; commits: 69f75c6629c4, b9acd9f7a010, 884dd1bb5511; files: scripts/native_eval/fleet.py, scripts/native_eval/run_job.py, scripts/native_eval/harnesses.py)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (22 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-01T17:56:46.557Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-01T19:16:46.445Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-02T13:16:59.157Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-02T19:15:27.999Z sha f5b31cb :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T20:38:31.284Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-02T22:18:15.271Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration
  • reviewed 2026-08-03T01:49:10.537Z sha f5b31cb :: found issues before merge. :: [P1] Preserve an upgrade path for retired tool-mode configuration | [P1] Translate legacy environment-based benchmark invocation
  • reviewed 2026-08-03T04:09:09.432Z sha f5b31cb :: found issues before merge. :: [P1] Preserve retired manifest tool-mode configuration | [P1] Translate the legacy environment tool-mode input

@vincentkoc
vincentkoc marked this pull request as ready for review July 29, 2026 20:56
@vincentkoc
vincentkoc requested a review from a team as a code owner July 29, 2026 20:56
@vincentkoc vincentkoc changed the title fix(native-eval): make OpenClaw Tool Search controls explicit fix(native-eval): benchmark genuine OpenClaw code mode Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(native-eval): make OpenClaw Tool Search controls explicit

1 participant